Hmm, would it be a bad idea to rewrite the enum values? Should we just reserve these values and start from 25?

Error: Enum value "18" on enum "IntegrationEnrollKind" changed name from "INTEGRATION_ENROLL_KIND_SERVICENOW" to "INTEGRATION_ENROLL_KIND_MACHINE_ID_AWS".
Error: Enum value "19" on enum "IntegrationEnrollKind" changed name from "INTEGRATION_ENROLL_KIND_ENTRA_ID" to "INTEGRATION_ENROLL_KIND_MACHINE_ID_GCP".
Error: Enum value "20" on enum "IntegrationEnrollKind" changed name from "INTEGRATION_ENROLL_KIND_DATADOG_INCIDENT_MANAGEMENT" to "INTEGRATION_ENROLL_KIND_MACHINE_ID_AZURE".
Error: buf found 3 breaking changes.

Anyone know if we need to maintain the two separate enums instead of using just one?

IIRC we need to have it in both the usage reporter service and the prehog service, so both enums are necessary. It looks like they were intended to be kept in sync with each other. We should definitely add a comment on both enums that this is the case.

All that really matters though is that we can map the usage event enum to the prehog enum during prehog processing:

Since this is no longer a 1-1 mapping, we just need to add some mapping logic for the out-of-sync values:

func integrationEnrollMetadataToPrehog(u *usageeventsv1.IntegrationEnrollMetadata, userMD UserMetadata) *prehogv1a.IntegrationEnrollMetadata {
	// Some enums are out of sync and need to be mapped manually
	var prehogKind prehogv1a.IntegrationEnrollKind
	switch u.Kind {
		case INTEGRATION_ENROLL_KIND_SERVICENOW:
			prehogKind = prehogv1a.INTEGRATION_ENROLL_KIND_SERVICENOW
		case INTEGRATION_ENROLL_KIND_ENTRA_ID:
			prehogKind = prehogv1a.INTEGRATION_ENROLL_KIND_ENTRA_ID
		case INTEGRATION_ENROLL_KIND_DATADOG_INCIDENT_MANAGEMENT:
			prehogKind = prehogv1a.INTEGRATION_ENROLL_KIND_DATADOG_INCIDENT_MANAGEMENT

                ### other cases

		default:
			prehogKind = prehogv1a.IntegrationEnrollKind(u.Kind)
	}

	return &prehogv1a.IntegrationEnrollMetadata{
		Id:       u.Id,
		Kind:     prehogKind,
		UserName: userMD.Username,
	}
}

And if you want to get these back in sync, we could reserve all of the out-of sync enums and just start counting from 26 again like you said. Then we'd just need to maintain some backwards compatibility mapping in the logic above. But IMO this probably isn't worth it, we'll be back in sync for future enums anyways and we'll need to keep this mapping logic indefinitely anyways, right?

@strideynet any idea what happened here?

It looks like you added some new enum variants to the cloud proto in https://github.com/gravitational/cloud/pull/6792/ with a note that the RPC handler converts this enum to a string representation, so no changes are actually needed to the handling code, but the variants were never added on the Teleport side. Was this intentional?

Edit: I see they were added to Teleport in #34985 but only in the teleport.proto and not in the usageevents.proto. What a footgun we have here.

Edit: I see they were added to Teleport in #34985 but only in the teleport.proto and not in the usageevents.proto. What a footgun we have here.

Ah yeah wow - that'll be the root of the issue here. This seems like a mistake that's a little too easy to mistake - is there a reason that one of these protos can't just reference the other one as a dependency ?

is there a reason that one of these protos can't just reference the other one as a dependency ?

Sorry, I let this PR get stale. I'll go ahead and merge this for now to fix metrics collection. We can revisit this topic in a separate PR.

@bernardjkim See the table below for backport results.